Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix intellijinit on Windows #134

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix intellijinit on Windows #134

wants to merge 3 commits into from

Conversation

okutane
Copy link
Contributor

@okutane okutane commented Apr 26, 2017 •
edited
Loading

Hi! I 've just tried to run 'mx.cmd intellijinit' for sulong project on windows and got following error:

  File "T:\dev\sulong-dev\mx\/mx.py", line 15330, in <module>
    main()
  File "T:\dev\sulong-dev\mx\/mx.py", line 15314, in main
    retcode = c(command_args)
  File "T:\dev\sulong-dev\mx\/mx.py", line 12737, in intellijinit
    _intellij_suite(args, suite, refreshOnly, mx_python_modules, java_modules, suite != primary_suite())
  File "T:\dev\sulong-dev\mx\/mx.py", line 12959, in _intellij_suite
    path = os.path.relpath(library.get_path(True), s.dir)
  File "C:\Python27\lib\ntpath.py", line 529, in relpath
    % (path_prefix, start_prefix))
ValueError: path is on drive C:, start on drive T:

The problem occured because I've put my dev directory on T: drive and temp files were under my user directory on C:

I've implemented the minimal fix for my scenario which shouldn't break anything and afterwards reviewed neighboorhood code and added few similar fixes "just in case".

This is my first PR to mx and first PR in Python, so please give any necessary feedback on how to improve it and get it merged. :)

@okutane
Copy link
Contributor Author

okutane commented Apr 26, 2017

I've tried to also test eclipseinit and netbeansinit, but there were unrelated problem (which maybe occured before relpath related problem could have happened), so no fixes there for now.

zapster added a commit to zapster/mx that referenced this pull request Jul 1, 2017
@zapster
Merge pull request graalvm#134 in G/mx from ~JOSEF.E.EISL_ORACLE.COM/…
e78ce45 
…mx:mx-benchmark-conv-value to master

* commit '7d08ef6102b0d103742855f93a08d3a94dfc0e09':
  Bump version to 5.33.0
  mx benchmark: add max_string_field_length and cropping functions
  mx benchmark: add support for custom value formatting functions
@okutane
Copy link
Contributor Author

okutane commented Mar 8, 2018

Hello! Should this one be closed or something? :)

@gilles-duboscq
Copy link
Member

Hi, This looks good to me.
What about maybe_relpath as a function name? I think it would be clearer about the fact that it should only be used in cases where a non-relative path is acceptable.

@JornVernee
Copy link
Contributor

Currently also running into this problem; using that safe_relpath function as a replacement for os.path.relpath seems to do the trick.

@okutane
Copy link
Contributor Author

okutane commented Nov 7, 2018

Currently I don't have time to update the PR, can someone else do it? :)

@JornVernee JornVernee mentioned this pull request Nov 8, 2018
Merged
@JornVernee
Copy link
Contributor

JornVernee commented Nov 8, 2018 •
edited
Loading

Currently I don't have time to update the PR, can someone else do it? :)

I can not edit this PR (because I don't have push access to your fork), so I have created a separated one: #164

Is that OK with you?

@gilles-duboscq
Copy link
Member

#164 was merged so i think this could be closed. Does it work for you @okutane?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gilles-duboscq gilles-duboscq Awaiting requested review from gilles-duboscq

At least 1 approving review is required to merge this pull request.

Assignees

@gilles-duboscq gilles-duboscq

Labels
oca-signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@okutane @gilles-duboscq @JornVernee @graalvmbot